Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cache-control for resources #60

Merged
merged 4 commits into from
Nov 2, 2022
Merged

Add cache-control for resources #60

merged 4 commits into from
Nov 2, 2022

Conversation

donpui
Copy link
Contributor

@donpui donpui commented Oct 26, 2022

As we are not using any CDN, lets add some cache-control for static resource management, to speed up a little bit of Winden.app resources

@donpui donpui marked this pull request as ready for review October 26, 2022 05:56
@donpui donpui requested review from wuan and JustusFT October 26, 2022 05:56
@donpui
Copy link
Contributor Author

donpui commented Oct 26, 2022

It works on stage.winden.app

@wuan
Copy link
Contributor

wuan commented Oct 26, 2022

Regarding the caching of code I have seen the following pattern in the past: Cached code elements use a filename which contains a hash value derived from the content, e. g. main_2c0bfa304ad5010b49a8ee9d76fe39d8.js. The only file which is not cached is the central index.html.

This has the advantage, that you can cache code (JavaScript and WASM) indefinitely while still having the possibility to perform an update which will roll out to all users as fast as possible.

I assume that this would require updating the way we build our application, but it might be useful.

Copy link
Contributor

@wuan wuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, only question I have: What happens when we update js/wasm? Is the change propagating immediately or do we need to wait until the data expires?

@meejah
Copy link
Collaborator

meejah commented Oct 26, 2022

What happens when we update js/wasm? Is the change propagating immediately or do we need to wait until the data expires?

From a quick scan I believe a user will have to wait at least 1 day to see the change (unless they e.g. "ctrl-shift-R" in Firefox to force a re-load). I think @wuan's first suggestion is good here -- so maybe we should turn off caching on JS until that (or similar) is implemented?

I believe webpack does this by default, but I don't know how we're packaging JS.

@donpui
Copy link
Contributor Author

donpui commented Oct 27, 2022

What happens when we update js/wasm? Is the change propagating immediately or do we need to wait until the data expires?

From a quick scan I believe a user will have to wait at least 1 day to see the change (unless they e.g. "ctrl-shift-R" in Firefox to force a re-load). I think @wuan's first suggestion is good here -- so maybe we should turn off caching on JS until that (or similar) is implemented?

I believe webpack does this by default, but I don't know how we're packaging JS.

We use gulp, but I see there are some webpack stuff too. However we don't have hash filename for wasm or js put in place. Maybe we can do that? https://www.npmjs.com/package/gulp-hash-filename @JustusFT what is your opinion here?

@JustusFT
Copy link
Contributor

JustusFT commented Nov 1, 2022

We may be able to automatically hash the wasm file if we can use a webpack loader to build it (maybe this?). Otherwise we can use gulp-hash-filename as long as we have a way to keep the fetch statement updated as well

@donpui
Copy link
Contributor Author

donpui commented Nov 1, 2022

I will update cache-control to remove js and wasm till find option to generate them with hash filenames

@donpui
Copy link
Contributor Author

donpui commented Nov 1, 2022

For further improvement, this task: #65

@donpui donpui merged commit 2653577 into main Nov 2, 2022
@meejah meejah deleted the enable-cache-control branch November 2, 2022 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants